Preliminary support for Matter cover by hidaris · Pull Request #90262 · home-assistant/core

hidaris · 2023-03-25T11:35:34Z

Proposed change

This PR adds matter cover control feature. Curtain tilt support has not been added yet.

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue: aqara m2 issue matter-js/python-matter-server#225
Link to documentation pull request: Add matter cover platform home-assistant.io#26857

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

home-assistant · 2023-03-25T11:35:37Z

Hi @hidaris

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

home-assistant · 2023-03-25T11:35:40Z

Hey there @home-assistant/matter, mind taking a look at this pull request as it has been labeled with an integration (matter) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of matter can trigger bot actions by commenting:

@home-assistant close Closes the pull request.
@home-assistant rename Awesome new title Renames the pull request.
@home-assistant reopen Reopen the pull request.
@home-assistant unassign matter Removes the current integration label and assignees on the pull request, add the integration domain after the command.

MartinHjelmare

Please also add tests. See the other platforms for examples of tests using the diagnostic dumps stored as fixtures.

home-assistant · 2023-03-27T09:53:49Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

marcelveldt · 2023-03-29T12:19:13Z

Hi @hidaris do you have time/interest to address the feedback comments or may/can I help you finishing it ?

hidaris · 2023-03-29T14:30:17Z

Hi @hidaris do you have time/interest to address the feedback comments or may/can I help you finishing it ?

Most of the feedbacks I've fixed locally, but because of the network in my region, the tests I've added don't have environmental testing at the moment, and I'm working on that...

hidaris · 2023-03-29T16:40:21Z

I added the test, but it keeps telling me I don't have the entity id, I used flag --snapshot-update, but it didn't solve the problem, can anyone tell me what's wrong?

MartinHjelmare · 2023-03-30T07:27:49Z

If you address the lint issues we can run CI again and see the error message of the pytest tests, and figure out what's wrong. I'm guessing the entity_id may be incorrect.

hidaris · 2023-03-30T07:39:21Z

If you address the lint issues, we can rerun CI and see the pytest error message and figure out what is wrong. I suspect that the entity_id might be wrong.

I just pushed the fix

MartinHjelmare · 2023-03-30T07:44:02Z

Please also address the lint issues: ruff, isort and pylint
Click on the jobs to view the logs and what's wrong.

hidaris · 2023-03-30T10:37:55Z

Please also address the lint issues: ruff, isort and pylint Click on the jobs to view the logs and what's wrong.

I adjusted the ruff and isort issue, pylint says I have a problem with intflag, I'm not sure if that's a problem

MartinHjelmare · 2023-03-30T14:57:49Z

The entity_id seems to be cover.longan_link_wncv_da01. It's logged if you read the CI log.

MartinHjelmare · 2023-03-30T18:04:40Z

Tests are still failing but for another reason now. Not sure what's wrong.

hidaris · 2023-03-30T19:27:55Z

Tests are still failing but for another reason now. Not sure what's wrong.

It was a problem with the test fixture, available was not set to true, and then I found that the window state tests added to the previous test depended on the operational status of the subscription, which was not updated correctly during the mock, so I removed them.

MartinHjelmare · 2023-03-31T05:33:51Z

Can't we set the operational status attribute too in the test so we can test all possible states?

hidaris · 2023-03-31T10:26:38Z

Can't we set the operational status attribute too in the test so we can test all possible states?

Yes, I added it

hidaris · 2023-03-31T12:26:09Z

CI says docs-missing, does this need to be fixed?

MartinHjelmare · 2023-03-31T15:02:30Z

Yes, please link a docs PR with an addition of the new platform to the ha_platforms front matter at the top of the page.

https://www.home-assistant.io/integrations/matter/

hidaris · 2023-04-04T06:39:53Z

Yes, please link a docs PR with an addition of the new platform to the ha_platforms front matter at the top of the page.

https://www.home-assistant.io/integrations/matter/

I just updated the documentation home-assistant/home-assistant.io#26857

…d code

MartinHjelmare

Looks good!

marcelveldt

Awesome, thanks for your contribution !

hidaris · 2023-04-04T12:09:55Z

@marcelveldt Just wondering if you're working on the Thermostat cluster? I've got some free time next week and was thinking of giving it a shot.

marcelveldt · 2023-04-04T12:14:04Z

@marcelveldt Just wondering if you're working on the Thermostat cluster? I've got some free time next week and was thinking of giving it a shot.

Yes, I am. I'll drop a PR for that this week.
Which device do you have/own ? Because thermostat/climate devices are pretty extended so would be good to test a few devices

hidaris · 2023-04-04T15:27:53Z

@marcelveldt Just wondering if you're working on the Thermostat cluster? I've got some free time next week and was thinking of giving it a shot.

Yes, I am. I'll drop a PR for that this week. Which device do you have/own ? Because thermostat/climate devices are pretty extended so would be good to test a few devices

Looking forward to it! As a non-native English speaker, my description of the device may not be entirely accurate, but to me it is a control panel for central air conditioning. In the matter specification, it corresponds to the thermostat under HVAC.

preliminary support for Matter cover

fcd769c

hidaris requested a review from a team as a code owner March 25, 2023 11:35

home-assistant Bot added cla-needed integration: matter new-feature new-platform labels Mar 25, 2023

home-assistant Bot added the Quality Scale: No score label Mar 25, 2023

home-assistant Bot added cla-recheck cla-signed and removed cla-recheck cla-needed labels Mar 25, 2023

hidaris mentioned this pull request Mar 25, 2023

aqara m2 issue matter-js/python-matter-server#225

Closed

MartinHjelmare requested changes Mar 27, 2023

View reviewed changes

home-assistant Bot marked this pull request as draft March 27, 2023 09:53

MartinHjelmare reviewed Mar 27, 2023

View reviewed changes

Comment thread homeassistant/components/matter/cover.py Outdated

add matter cover test, tweak some code structure

ec0dc90

rename matter cover test case

d1528f6

MartinHjelmare reviewed Mar 30, 2023

View reviewed changes

Comment thread homeassistant/components/matter/cover.py Outdated

Comment thread homeassistant/components/matter/cover.py Outdated

change operational status to snake case, reorder match case

0298fb0

fix lint problem in matter cover

ff5b61e

fix cover entity id in tests

b1b8bf3

MartinHjelmare reviewed Mar 30, 2023

View reviewed changes

Comment thread homeassistant/components/matter/discovery.py Outdated

hidaris added 2 commits March 31, 2023 03:21

fix matter cover tests

aca0091

keep discovery schemas sorted

65ee421

recover matter cover state tests

5e5fae3

This was referenced Apr 4, 2023

Add cover to matter platform, update some outdated information home-assistant/home-assistant.io#26856

Closed

Add matter cover platform home-assistant/home-assistant.io#26857

Merged

marcelveldt reviewed Apr 4, 2023

View reviewed changes

Comment thread homeassistant/components/matter/cover.py Outdated

marcelveldt reviewed Apr 4, 2023

View reviewed changes

Comment thread homeassistant/components/matter/cover.py Outdated

marcelveldt reviewed Apr 4, 2023

View reviewed changes

Comment thread homeassistant/components/matter/cover.py Outdated

Adjust the coding style related to OperationalStatus and remove unuse…

c60379b

…d code

MartinHjelmare approved these changes Apr 4, 2023

View reviewed changes

hidaris marked this pull request as ready for review April 4, 2023 09:57

home-assistant Bot requested a review from MartinHjelmare April 4, 2023 09:57

marcelveldt approved these changes Apr 4, 2023

View reviewed changes

marcelveldt merged commit a9e14cd into home-assistant:dev Apr 4, 2023

emontnemery mentioned this pull request Apr 4, 2023

Run prettier on matter fixture #90784

Merged

20 tasks

github-actions Bot locked and limited conversation to collaborators Apr 5, 2023

frenck added the noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) label Apr 24, 2023

Uh oh!

Conversation

hidaris commented Mar 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed change

Type of change

Additional information

Checklist

Uh oh!

home-assistant Bot commented Mar 25, 2023

Uh oh!

home-assistant Bot commented Mar 25, 2023

Uh oh!

MartinHjelmare left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

home-assistant Bot commented Mar 27, 2023

Uh oh!

Uh oh!

marcelveldt commented Mar 29, 2023

Uh oh!

hidaris commented Mar 29, 2023

Uh oh!

hidaris commented Mar 29, 2023

Uh oh!

Uh oh!

Uh oh!

MartinHjelmare commented Mar 30, 2023

Uh oh!

hidaris commented Mar 30, 2023

Uh oh!

MartinHjelmare commented Mar 30, 2023

Uh oh!

hidaris commented Mar 30, 2023

Uh oh!

MartinHjelmare commented Mar 30, 2023

Uh oh!

Uh oh!

MartinHjelmare commented Mar 30, 2023

Uh oh!

hidaris commented Mar 30, 2023

Uh oh!

MartinHjelmare commented Mar 31, 2023

Uh oh!

hidaris commented Mar 31, 2023

Uh oh!

hidaris commented Mar 31, 2023

Uh oh!

MartinHjelmare commented Mar 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hidaris commented Apr 4, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MartinHjelmare left a comment

Choose a reason for hiding this comment

Uh oh!

marcelveldt left a comment

Choose a reason for hiding this comment

Uh oh!

hidaris commented Apr 4, 2023

Uh oh!

marcelveldt commented Apr 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hidaris commented Apr 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

hidaris commented Mar 25, 2023 •

edited

Loading

MartinHjelmare commented Mar 31, 2023 •

edited

Loading

marcelveldt commented Apr 4, 2023 •

edited

Loading